Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parentheses to nested comparison expressions #1813

Merged
merged 1 commit into from
Jun 29, 2024

Conversation

Jacky720
Copy link
Contributor

Description

Minor fix for a bug reported on Discord where the code x -= ((1 + statetime) < (2 + statetime)) < 4 would decompile as x -= (1 + statetime) < (2 + statetime) < 4.

Caveats

Only the possible exceptions where parentheses might be helpful to other, yet rarer, expressions, which would have already been decompiling incorrectly.

Notes

N/A

@CST1229
Copy link
Contributor

CST1229 commented Jun 24, 2024

a < b < c is valid GML syntax (it's accepted by GameMaker's compiler), so I think it might be better/saves parentheses to instead allow multiple comparison operators next to eachother in the compiler (like it already is possible with math), which I already did for UTMTCE. Maybe I should PR that.

@Jacky720
Copy link
Contributor Author

What does GameMaker's compiler produce in this case? If it's like Python, it would be a < b && b < c, but if it's actually producing (a < b) < c as in turning the first comparison from boolean to number, we should decompile that.

@Jacky720
Copy link
Contributor Author

Yeah, I just spun up a dummy project, that's exactly what it's doing.
image
"a < b" is false, 0, and 0 < c. (a < b) < c is the opposite of the code's intent, of course, but consistent with the new decompilation (and the mod tool compiler can handle it).

@Miepee
Copy link
Contributor

Miepee commented Jun 29, 2024

Even though it is a compiler bug, let's just always force parattheses for now on. Makes that code way more readable and prevents user bugs.

@Miepee Miepee merged commit 43a0c3f into UnderminersTeam:master Jun 29, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants